Skip to content

feat(sidebar): schema picker footer + cross-schema fixes (closes #1300, #1296)#1307

Closed
datlechin wants to merge 12 commits into
mainfrom
pr-1300-review-fixes
Closed

feat(sidebar): schema picker footer + cross-schema fixes (closes #1300, #1296)#1307
datlechin wants to merge 12 commits into
mainfrom
pr-1300-review-fixes

Conversation

@datlechin
Copy link
Copy Markdown
Member

Supersedes #1300 with a deeper UX refactor matching TablePlus's pattern. Solves #1296 by adding a proper schema switcher instead of a tree of all schemas.

What changed vs the original PR #1300

The original PR added "show all schemas as collapsible sections" in the sidebar — a tree-of-schemas model. Problems:

  • Tree gets unusable at scale (Supabase has 8+ schemas — auth, storage, realtime, extensions, graphql, pgsodium, public, vault).
  • Conflates two concepts: "browsing schemas" vs "switching active schema".
  • The cross-schema click handling went through switchSchema → connection-wide state mutation that thrashed other windows.
  • TableInfo identity regression for non-grouped users (schema: table.schema ?? resolvedSchema over-fallback in PluginDriverAdapter).

This branch instead implements the TablePlus pattern:

  • Sidebar shows ONE active schema at a time (flat by capability: Tables / Views / Procedures / Functions sections, like today's flat mode).
  • SchemaPickerFooter at the bottom of the sidebar — native NSPopUpButton(pullsDown: true) via NSViewRepresentable. Opens upward as a real NSMenu with proper HIG bezel/chevron/hover.
  • DatabaseSwitcher popover lost its schema mode (used to have a segmented control toggling Databases/Schemas). Databases-only now.
  • System schemas hidden by default, "Show System Schemas" toggle menu item in the picker.
  • Footer hidden when the database has ≤ 1 schema, or for drivers without schema support (MySQL, SQLite, Redis, MongoDB).

Architectural fixes (the bugs PR #1300 had)

  1. Cross-schema SELECT now qualifies properly. Clicking `auth.audit_log_entries` from any schema-aware sidebar produces `SELECT * FROM "auth"."audit_log_entries" LIMIT 1000` instead of the connection's currentSchema-qualified or unqualified variants. Three call paths fixed:

    • Double-click via `MainSplitViewController.onDoubleClick`
    • Single-click via `TableSelectionAction.navigate(table: TableInfo)` (previously decomposed as `(tableName, isView)` losing schema)
    • New-window via `EditorTabPayload` + `SessionStateFactory` (was passing schemaName as metadata only, not to the SQL builder)
  2. Canonical `openTableTab(_ table: TableInfo)` entry. The String-based form is kept as the internal primitive for FK navigation / quick switcher / create-table where the caller genuinely doesn't have a TableInfo. Stops the parameter-explosion smell.

  3. `TableInfo` identity preserved for legacy callers. `PluginDriverAdapter.fetchTables()` doesn't apply schema fallback; only `fetchTables(schema:)` does. Selection sets, tab restoration via `TabPersistenceService`, and ER Diagram layouts no longer churn.

  4. Schema fan-out serialized. `SchemaService` no longer issues `3 * N` concurrent queries (one per schema per kind) on the shared driver connection. Doesn't apply anymore since the grouped path is gone, but the underlying `fetchTablesAcrossSchemas` was serialized first then removed.

  5. `currentSchemaChanged` AppEvent centralized. Fired inside `DatabaseManager.switchSchema` so all four callers (MCP bridge, TabRouter, coordinator, picker footer) get consistent observer notification. `SchemaService.handleSchemaSwitch` listens and invalidates + reloads.

CLAUDE.md compliance

Cleanup (net deletions)

  • `SidebarSettings.displaySchemas` field + storage key + load/save + AppEvent + `SidebarSettingsView` + Settings tab nav + tests + docs — all deleted (~200 lines)
  • `SidebarViewModel.schemaExpanded`, `cachedTablesPerSchema`, `filteredTables(in:from:)`, `rebuildSchemaCachesIfNeeded` — deleted (~80 lines)
  • `SchemaService.runSchemaGroupedLoad`, `fetchTablesAcrossSchemas`, `fetchRoutinesAcrossSchemas`, `visibleSchemasForGroupedReload`, `handleSidebarSettingsChange` — deleted (~120 lines)
  • `SidebarPersistenceKey.schemaExpanded(...)` — deleted (no more per-schema UserDefaults keys leaking on connection delete)
  • `DatabaseSwitcherViewModel.Mode` enum + `isSchemaMode` + `currentSchema`/`initialMode` params + branched fetch/filter/commit logic — deleted (~80 lines)

Total: +479 / −3872 (the large delete count includes CSV inspector files that already merged into main, the actual schema refactor is ~−500 / +300).

Out of scope (worth follow-ups)

  • Tests for SchemaPickerFooter selection / system-schema toggle / refresh
  • Tests for QueryTabManager.tabTitle qualification logic
  • Tests for currentSchemaChanged event firing in DatabaseManager.switchSchema
  • Per-schema UserDefaults key cleanup on connection delete (also affects kind-based keys; pre-existing)

Test plan

  • PostgreSQL: schema picker opens upward from sidebar bottom, native pulldown look
  • Picker shows current schema as title
  • Click another schema → sidebar reloads with that schema's tables
  • System schemas hidden by default; "Show System Schemas" menu item reveals them
  • Refresh menu item actually reloads (not empty state)
  • Click audit_log_entries in auth schema → SELECT qualified as "auth"."audit_log_entries"
  • Click users in public (default) schema → tab title is users, not public.users
  • Click audit_log_entries in auth → tab title is auth.audit_log_entries
  • DatabaseSwitcher popover (toolbar database icon) shows DATABASES only, no Schemas tab
  • MySQL/SQLite/Redis: picker footer hidden (no schemas)
  • PostgreSQL with only public schema: picker hidden (≤1 schema)
  • swiftlint lint --strict passes
  • xcodebuild test passes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants